Why is std::shared_ptr::unique() deprecated?
Asked Answered
A

4

37

What is the technical problem with std::shared_ptr::unique() that is the reason for its deprecation in C++17?

According to cppreference.com, std::shared_ptr::unique() is deprecated in C++17 as

this function is deprecated as of C++17 because use_count is only an approximation in multi-threaded environment.

I understand this to be true for use_count() > 1: While I'm holding a reference to it, someone else might simultaneously let go of his or create a new copy.

But if use_count() returns 1 (which is what I'm interested in when calling unique()) then there is no other thread that could change that value in a racy way, so I would expect that this should be safe:

if (myPtr && myPtr.unique()) {
    //Modify *myPtr
}

Results from my own search:

I found this document: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0521r0.html which proposes the deprecation in response to C++17 CD comment CA 14, but I couldn't find said comment itself.

As an alternative, that paper proposed adding some notes including the following:

Note: When multiple threads can affect the return value of use_count(), the result should be treated as approximate. In particular, use_count() == 1 does not imply that accesses through a previously destroyed shared_ptr have in any sense completed. — end note

I understand that this might be the case for the way use_count() is currently specified (due to the lack of guaranteed synchronization), but why was the resolution not just to specify such synchronization and hence make the above pattern safe? If there was a fundamental limitation that wouldn't allow such synchronization (or make it forbiddingly costly), then how is it possible to correctly implement the destructor?

Update:

I overlooked the obvious case presented by @alexeykuzmin0 and @rubenvb, because so far I only used unique() on instances of shared_ptr that were not accessible to other threads themselves. So there was no danger that that particular instance would get copied in a racy way.

I still would be interested to hear what exactly CA 14 was about, because I believe that all my use cases for unique() would work as long as it is guaranteed to synchronize with whatever happens to different shared_ptr instances on other threads. So it still seems like a useful tool to me, but I might overlook something fundamental here.

To illustrate what I have in mind, consider the following:

class MemoryCache {
public:
    MemoryCache(size_t size)
        : _cache(size)
    {
        for (auto& ptr : _cache) {
            ptr = std::make_shared<std::array<uint8_t, 256>>();
        }
    }

    // the returned chunk of memory might be passed to a different thread(s),
    // but the function is never accessed from two threads at the same time
    std::shared_ptr<std::array<uint8_t,256>> getChunk()
    {
        auto it = std::find_if(_cache.begin(), _cache.end(), [](auto& ptr) { return ptr.unique(); });
        if (it != _cache.end()) {
            //memory is no longer used by previous user, so it can be given to someone else
            return *it;
        } else {
            return{};
        }
    }
private:
    std::vector<std::shared_ptr<std::array<uint8_t, 256>>> _cache;
};

Is there anything wrong with it (if unique() would actually synchronize with the destructors of other copies)?

Ainsworth answered 14/12, 2016 at 12:10 Comment(9)
Why is 1 a special case? There might be another copy created after your call to unique and before you finish whatever you're doing.Degrading
@rubenvb: If use_count == 1, then there is - by defintion - no other thread that has a reference it could make a copy from.Ainsworth
@rubenvb: My mistake - unique is const, so another thread could make a copy without it being a data-raceAinsworth
const has nothing to do with it. If an object holding the shared_ptr is itself accessible from multiple threads, copies of the shared_ptr can be made regardless.Degrading
@rubenvb: If it wasn't const, then you'd have datarace when reading from the same instance simultaneously anywayAinsworth
const doesn't prevent data races.Degrading
@rubenvb: IIRC, the standard library guarantees that as long as you only call const methods on an object, there will be no data races.Ainsworth
@MikeMB, I was about to answer to your update but I have seen that a sentence in the answer of alexeykuzmin0 almost answer your update. Your are right, If each of your thread only use copies of a shared_ptr then unique() will work fine. In the paper you cite and in the answer of alexykuzmin0 multiple thread share a reference to a unique shared_ptr... In the answer of alexeykuzmin0 the sentence "The unique()=true means that no one has a shared_ptr pointing to the same memory... may say almost the same thing no?Phebephedra
Of course use_count was not deprecated, so you can continue using use_count()==1 so long as you remember it's racy.Spokesman
S
9

I think that P0521R0 solves potentially data race by misusing shared_ptr as inter-thread synchronization. It says use_count() returns unreliable refcount value, and so, unique() member function will be useless when multithreading.

int main() {
  int result = 0;
  auto sp1 = std::make_shared<int>(0);  // refcount: 1

  // Start another thread
  std::thread another_thread([&result, sp2 = sp1]{  // refcount: 1 -> 2
    result = 42;  // [W] store to result
    // [D] expire sp2 scope, and refcount: 2 -> 1
  });

  // Do multithreading stuff:
  //   Other threads may concurrently increment/decrement refcounf.

  if (sp1.unique()) {      // [U] refcount == 1?
    assert(result == 42);  // [R] read from result
    // This [R] read action cause data race w.r.t [W] write action.
  }

  another_thread.join();
  // Side note: thread termination and join() member function
  // have happens-before relationship, so [W] happens-before [R]
  // and there is no data race on following read action.
  assert(result == 42);
}

The member function unique() does not have any synchronization effect and there're no happens-before relationship from [D] shared_ptr's destructor to [U] calling unique(). So we cannot expect relationship [W] ⇒ [D] ⇒ [U] ⇒ [R] and [W] ⇒ [R]. ('⇒' denotes happens-before relationship).


EDITED: I found two related LWG Issues; LWG2434. shared_ptr::use_count() is efficient, LWG2776. shared_ptr unique() and use_count(). It is just a speculation, but WG21 Committee gives priority to the existing implementation of C++ Standard Library, so they codify its behavior in C++1z.

LWG2434 quote (emphasis mine):

shared_ptr and weak_ptr have Notes that their use_count() might be inefficient. This is an attempt to acknowledge reflinked implementations (which can be used by Loki smart pointers, for example). However, there aren't any shared_ptr implementations that use reflinking, especially after C++11 recognized the existence of multithreading. Everyone uses atomic refcounts, so use_count() is just an atomic load.

LWG2776 quote (emphasis mine):

The removal of the "debug only" restriction for use_count() and unique() in shared_ptr by LWG 2434 introduced a bug. In order for unique() to produce a useful and reliable value, it needs a synchronize clause to ensure that prior accesses through another reference are visible to the successful caller of unique(). Many current implementations use a relaxed load, and do not provide this guarantee, since it's not stated in the standard. For debug/hint usage that was OK. Without it the specification is unclear and probably misleading.

[...]

I would prefer to specify use_count() as only providing an unreliable hint of the actual count (another way of saying debug only). Or deprecate it, as JF suggested. We can't make use_count() reliable without adding substantially more fencing. We really don't want someone waiting for use_count() == 2 to determine that another thread got that far. And unfortunately, I don't think we currently say anything to make it clear that's a mistake.

This would imply that use_count() normally uses memory_order_relaxed, and unique is neither specified nor implemented in terms of use_count().

Stepparent answered 15/12, 2016 at 9:10 Comment(7)
That is actually an interesting point. Anybody who's using a shared pointer in the way your demonstration example does should probably get whipped, but there might be more complex patterns, where this assumption is made implicitly somewhere. I still would have preferred that they fix unique() by specifying a synchronization effect instead of deprecating it, but there might be performance implications that I don't see.Ainsworth
Thanks for digging up those commentsAinsworth
That is totally wrong: "so, unique() member function will be useless when multithreading". No, unique() is not useful for synchronization. That doesn't at all mean that it's "useless". I certainly hope you didn't really mean that, because then everything that cannot be used for synchronization would be "useless when multithreading".Meteorology
It seems that there is no data race in [R] with [W] because sp1.unique() == true only when another_thread is finished. Thread working with a copy of sp1, so at least two references to a pointer are exist and sp1.unique() cannot be true while thread is runningFrutescent
@user3514538 Your interpretation about C++ atomics is wrong, read LWG2776.Stepparent
C++ atomic relaxed access does not guarantee any ordering to another atomic access, even if they has control dependencies. For instance "C11 atomic variables and the kernel" explain similar situation.Stepparent
use_count() and unique() are truly meaningless in the presence of weak_ptr<> because lock() may increase use_count() other than through a shared_ptr<> by creating a new one! So the assumption sp.unique()==true means sp has acquired durable single ownership is just false with weak-pointers in play.Oldster
E
11

Consider the following code:

// global variable
std::shared_ptr<int> s = std::make_shared<int>();

// thread 1
if (s && s.unique()) {
    // modify *s
}

// thread 2
auto s2 = s;

Here we have a classic race condition: s2 may (or may not) be created as a copy of s in thread 2 while thread 1 is inside the if.

The unique() == true means that no one has a shared_ptr pointing to the same memory, but does not mean any other threads have no access to initial shared_ptr directly or through pointers or references.

Equestrienne answered 14/12, 2016 at 12:20 Comment(4)
Thank you. I don't know, why I overlooked that obvious case. I still don't see why the function should be deprecated, but at least the decision makes more sense to me now.Ainsworth
I updated my question a bit. If you have any thoughts on it, please feel free to share.Ainsworth
I don't understand why this answer has so many upvotes. s.unique() == true was never claimed to mean that no other threads have access to s.Spokesman
This is silly. Thread 1 could be executing anything that modifies s and this would be a race. This is nothing about unique and simply about the fact that you really don't want threads to share instances of shared_ptr but instead to each have their own instance that references the same underlying object.Oneiromancy
S
9

I think that P0521R0 solves potentially data race by misusing shared_ptr as inter-thread synchronization. It says use_count() returns unreliable refcount value, and so, unique() member function will be useless when multithreading.

int main() {
  int result = 0;
  auto sp1 = std::make_shared<int>(0);  // refcount: 1

  // Start another thread
  std::thread another_thread([&result, sp2 = sp1]{  // refcount: 1 -> 2
    result = 42;  // [W] store to result
    // [D] expire sp2 scope, and refcount: 2 -> 1
  });

  // Do multithreading stuff:
  //   Other threads may concurrently increment/decrement refcounf.

  if (sp1.unique()) {      // [U] refcount == 1?
    assert(result == 42);  // [R] read from result
    // This [R] read action cause data race w.r.t [W] write action.
  }

  another_thread.join();
  // Side note: thread termination and join() member function
  // have happens-before relationship, so [W] happens-before [R]
  // and there is no data race on following read action.
  assert(result == 42);
}

The member function unique() does not have any synchronization effect and there're no happens-before relationship from [D] shared_ptr's destructor to [U] calling unique(). So we cannot expect relationship [W] ⇒ [D] ⇒ [U] ⇒ [R] and [W] ⇒ [R]. ('⇒' denotes happens-before relationship).


EDITED: I found two related LWG Issues; LWG2434. shared_ptr::use_count() is efficient, LWG2776. shared_ptr unique() and use_count(). It is just a speculation, but WG21 Committee gives priority to the existing implementation of C++ Standard Library, so they codify its behavior in C++1z.

LWG2434 quote (emphasis mine):

shared_ptr and weak_ptr have Notes that their use_count() might be inefficient. This is an attempt to acknowledge reflinked implementations (which can be used by Loki smart pointers, for example). However, there aren't any shared_ptr implementations that use reflinking, especially after C++11 recognized the existence of multithreading. Everyone uses atomic refcounts, so use_count() is just an atomic load.

LWG2776 quote (emphasis mine):

The removal of the "debug only" restriction for use_count() and unique() in shared_ptr by LWG 2434 introduced a bug. In order for unique() to produce a useful and reliable value, it needs a synchronize clause to ensure that prior accesses through another reference are visible to the successful caller of unique(). Many current implementations use a relaxed load, and do not provide this guarantee, since it's not stated in the standard. For debug/hint usage that was OK. Without it the specification is unclear and probably misleading.

[...]

I would prefer to specify use_count() as only providing an unreliable hint of the actual count (another way of saying debug only). Or deprecate it, as JF suggested. We can't make use_count() reliable without adding substantially more fencing. We really don't want someone waiting for use_count() == 2 to determine that another thread got that far. And unfortunately, I don't think we currently say anything to make it clear that's a mistake.

This would imply that use_count() normally uses memory_order_relaxed, and unique is neither specified nor implemented in terms of use_count().

Stepparent answered 15/12, 2016 at 9:10 Comment(7)
That is actually an interesting point. Anybody who's using a shared pointer in the way your demonstration example does should probably get whipped, but there might be more complex patterns, where this assumption is made implicitly somewhere. I still would have preferred that they fix unique() by specifying a synchronization effect instead of deprecating it, but there might be performance implications that I don't see.Ainsworth
Thanks for digging up those commentsAinsworth
That is totally wrong: "so, unique() member function will be useless when multithreading". No, unique() is not useful for synchronization. That doesn't at all mean that it's "useless". I certainly hope you didn't really mean that, because then everything that cannot be used for synchronization would be "useless when multithreading".Meteorology
It seems that there is no data race in [R] with [W] because sp1.unique() == true only when another_thread is finished. Thread working with a copy of sp1, so at least two references to a pointer are exist and sp1.unique() cannot be true while thread is runningFrutescent
@user3514538 Your interpretation about C++ atomics is wrong, read LWG2776.Stepparent
C++ atomic relaxed access does not guarantee any ordering to another atomic access, even if they has control dependencies. For instance "C11 atomic variables and the kernel" explain similar situation.Stepparent
use_count() and unique() are truly meaningless in the presence of weak_ptr<> because lock() may increase use_count() other than through a shared_ptr<> by creating a new one! So the assumption sp.unique()==true means sp has acquired durable single ownership is just false with weak-pointers in play.Oldster
C
4

For your viewing pleasure: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0488r0.pdf

This document contains all NB (National Body) comments for the Issaquah meeting. CA 14 reads:

The removal of the "debug only" restriction for use_count() and unique() in shared_ptr introduced a bug: in order for unique() to produce a useful and reliable value, it needs a synchronize clause to ensure that prior accesses through another reference are visible to the successful caller of unique(). Many current implementations use a relaxed load, and do not provide this guarantee, since it's not stated in the Standard. For debug/hint usage that was OK. Without it the specification is unclear and misleading.

Concerned answered 14/12, 2016 at 18:1 Comment(7)
Thank you very much - I wonder why the commitee didn't take the comment's first proposed solution: "A solution could make unique() use memory_order_acquire, and specifying that reference count decrement operations synchronize with unique()."Ainsworth
@Ainsworth I am afraid my searching did not uncover anything that might shed light on their motivation. The information may exist, but it's not a low hanging fruit.Concerned
The synchronization needed for this would pessimise some fairly common operations (like copying), to enable a (ab-)use of unique/use_count which was never intended anyway.Sommer
@FabioFracassi: can you go into a little more detail here? Afaik for unique to work in my example the refcount increment wouldn't have to synchronize with anything (teh decrement would however).Ainsworth
Not really, I did not attend that discussion.Sommer
@Ainsworth Thread 1 stores the result of getChunk in x. Thread 2 copies x into y. Thread 1 destroys x. Thread 3 copies y into z. Thread 2 destroys y. The increment in thread 3 might not be seen by unique in thread 1.Spokesman
@Oktalist:AFAIk, even relaxed loads on atomics preserve modification order and are not allowed to produce values out of thin air (en.cppreference.com/w/cpp/atomic/memory_order#Relaxed_ordering). There has to be a synchronization barrier between the increment in thread 3 and the destruction of y in thread 2, because otherwise you would have a data race on y anyway. Hence the increment happens before the decrement, hence the ref_count never falls below 2 (even from the perspective of thread 1).Ainsworth
C
1

The existence of std::enable_shared_from_this is the trouble maker to make any interesting use of unique(). Indeed, std::enable_shared_from_this allows to make a new shared_ptr from a raw pointer, from any thread. This means unique() can never be a guarantee for anything.

But consider another library... Though this is not about shared_ptr, in Qt, there is an internal method called isDetached() with (almost) the same implementation as unique(). It is used for some quite useful optimization purpose: when true, the pointed object can be mutated without performing a "copy-on-write" operation. Indeed, once unique, a managed resource can't become shared by an action originating from another thread. The same pattern would be possible with shared_ptr if enable_shared_from_this did not exist.

This is why IMHO, unique() has been removed from C++20: misleading.

Calipee answered 7/8, 2021 at 14:51 Comment(5)
This applies equally to any weak_ptr (which is what enable_shared_from_this ultimately stores), since unique doesn't count weak references as "uses".Steffen
I don't der the connection. Just because a object is managed by a shared_ptr doesn't mean it inherits from enable_shared_from_this.Ainsworth
@MikeMB: this is right, but I quote 'enable_shared_from_this' to justify that 'unique()' can't be a guarantee for anything, because the APIs from shared_ptr could alter the reference counter in different threads, both to increment or decrement.Calipee
Yes, and what I'm saying is that your argument applies to std::shared_ptr<derived_from_enable_shared_from_this> but not for any other shared_ptr<T>Ainsworth
I guess the main reason is that validity of unique does not even exceed the line within which unique is called; check in thread A, then suspend A, then increment in thread B, then suspend B and resume A; and this was not even concurrent.Conspiracy

© 2022 - 2024 — McMap. All rights reserved.